-
Couldn't load subscription status.
- Fork 25.6k
Store arrays offsets for scaled float fields natively with synthetic source #125793
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Store arrays offsets for scaled float fields natively with synthetic source #125793
Conversation
|
Hi @jordan-powers, I've created a changelog YAML for you. |
|
Pinging @elastic/es-storage-engine (Team:StorageEngine) |
11f898f to
ba8e223
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A few nits, LGTM 👍
|
|
||
| @Override | ||
| protected void minimalMapping(XContentBuilder b) throws IOException { | ||
| b.field("type", "scaled_float").field("scaling_factor", 10.0); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe turn the scaling factor into a constant here?
|
|
||
| @Override | ||
| protected String getFieldTypeName() { | ||
| return null; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why does this return null here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I see, because minimalMapping(...) gets overwritten here. So actually this method should never be invoked. Maybe add an assert false here? So it fails quick if this gets invoked?
|
|
||
| @Override | ||
| protected void minimalMapping(XContentBuilder b) throws IOException { | ||
| b.field("type", "scaled_float").field("scaling_factor", 10.0); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe turn the scaling factor into a constant here?
💔 Backport failed
You can use sqren/backport to manually backport by running |
💚 All backports created successfully
Questions ?Please refer to the Backport tool documentation |
…source (elastic#125793) This patch builds on the work in elastic#113757, elastic#122999, elastic#124594, elastic#125529, and elastic#125709 to natively store array offsets for scaled float fields instead of falling back to ignored source when synthetic_source_keep: arrays.
…source (#125793) (#125891) This patch builds on the work in #113757, #122999, #124594, #125529, and #125709 to natively store array offsets for scaled float fields instead of falling back to ignored source when synthetic_source_keep: arrays. (cherry picked from commit 71e74bd) # Conflicts: # server/src/main/java/org/elasticsearch/index/IndexVersions.java
This patch builds on the work in #113757, #122999, #124594, #125529, and #125709 to natively store array offsets for scaled float fields instead of falling back to ignored source when
synthetic_source_keep: arrays.